bootstrap: include bootstrapped Environment in builtin snapshot #32984
Closed
joyeecheung wants to merge 13 commits intonodejs:masterfrom
Closed
bootstrap: include bootstrapped Environment in builtin snapshot #32984joyeecheung wants to merge 13 commits intonodejs:masterfrom
joyeecheung wants to merge 13 commits intonodejs:masterfrom
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This was the WIP I had been working on to give my updates in #17058 and promised to post in #26382 (comment) back in last December - I was not planning to open this until I get the issues mentioned in #17058 (comment) fixed in V8, but then #32761 came along with a significantly different design. So I rebased this against master and expanded the snapshot to include
bootstrap/node.jsfor comparison, and post it here to demonstrate the alternative design decisions I suggested in #32761.Similar to the MVP approach taken by #27321, this now contains only the essential code for getting builtin snapshot shipped in Node.js core - I tried my best to split the snapshot integration into smaller commits that do this step by step. User land snapshot integration would need more yaks shaved to move forward due to the natural constraints of the snapshots, so I left a few comments in the places where code need to be expanded when we do implement user land snapshot integration. We'll need to give more thoughts take care of the uncertainty of the object graph brought by user land snapshotting - in particular, how to properly reset the connection between JS and C++ states, as well as the C++ states and the external system states.
This also includes some new ideas I got after the discussions in #32761.
I've recorded the background and analysis of different options for these design decisions in https://docs.google.com/document/d/15bu038I36oILq5t4Qju1sS2nKudVB6NSGWz00oD48Q8/edit?usp=sharing some of them are implemented here, some in #32761. I think it might be better to keep design discussions in the doc instead of in PRs - so that we focus more on the pros and cons of these options from a higher level. A lot of the decisions are influenced by the previous discussions and prototypes in #17058
cc @addaleax (thanks for the patience for going over the design options with me and the inspiration for some of the ideas here!)
src: split the main context initialization from Environemnt ctor
So that it's possible to create an Environment not yet attached
to any V8 context. We'll use this to deserialize the V8 context
before attaching an Environment to it.
src: add an ExternalReferenceRegistry class
Add an ExternalReferenceRegistry class for registering static
external references.
To register the external JS to C++ references created in a binding
(e.g. when a FunctionTemplate is created):
Add the binding name (same as the id used for
internalBinding()and
NODE_MODULE_CONTEXT_AWARE_INTERNAL) toEXTERNAL_REFERENCE_BINDING_LISTinsrc/node_external_reference.h.In the file where the binding is implemented, create a registration
function to register the static C++ references (e.g. the C++
functions in
v8::FunctionCallbackassociated with the functiontemplates), like this:
At the end of the file where
NODE_MODULE_CONTEXT_AWARE_INTERNALisalso usually called, register the registration function with
tools: enable Node.js command line flags in node_mksnapshot
Pass the flags down to node_mksnapshot so that we can use them
when generating the snapshot (e.g. to debug or enable V8 flags)
src: snapshot Environment upon instantiation
This includes the initial Environment (without running bootstrap
scripts) into the builtin snapshot
src: make code cache test work with snapshots
Keep track of snapshotted modules in JS land, and move
bootstrap switches into StartExecution() so that
they are not included into part of the environment-independent
bootstrap process.
src: snapshot loaders
This runs
lib/internal/bootstrap/loaders.jsbefore creatingthe builtin snapshot and deserialize the loaders from the
snapshot in deserialization mode.
src: reset zero fill toggle at pre-execution
The connection between the JS land zero fill toggle and the
C++ one in the NodeArrayBufferAllocator gets lost if the toggle
is deserialized from the snapshot, because V8 owns the underlying
memory of this toggle. This resets the connection at pre-execution.
bootstrap: build fast APIs in pre-execution
Fast APIs need to work with ArrayBuffers which we need
to rebuild connections to after deserializing them
from the snapshot. For now, postpone their creation
until pre-execution to simplify the process.
lib: initialize instance members in class constructors
Since V8 snapshot does not currently support instance member
initialization, initialize them in ordianry class constructors
for now so that these classes can be included in the snapshot.
This may be reverted once
https://bugs.chromium.org/p/v8/issues/detail?id=10704
is fixed and backported.
src: snapshot node
This runs
lib/internal/bootstrap/node.jsbefore creatingthe builtin snapshot and deserialize the loaders from the
snapshot in deserialization mode.
make -j4 test(UNIX), orvcbuild test(Windows) passes